Skip to content

Removing card requirement for OC #4573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 30 commits into from
Aug 19, 2025
Merged

Removing card requirement for OC #4573

merged 30 commits into from
Aug 19, 2025

Conversation

wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Aug 11, 2025 •
edited
Loading

Fixes STRIPE-646

Changes proposed in this Pull Request:

In this PR I am removing the requirement for the credit card payment method when the Optimized Checkout is enabled. This way, merchants are allowed to use the feature with any payment method setup they want, without having to force CCs.

Testing instructions

  • Checkout to this branch on your test environment (fix/removing-card-requirement-for-oc)
  • Connect your Stripe account
  • Enable the Optimized Checkout
  • Disable the credit card payment method
  • Enable any other payment method(s)
  • As a shopper, add a product to your cart
  • Go to the checkout page
  • Confirm the OC container is rendered, showing the enabled payment methods inside of it
  • Confirm you are able to complete the purchase
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@wjrosa wjrosa self-assigned this Aug 11, 2025
@wjrosa wjrosa marked this pull request as ready for review August 11, 2025 21:30
@wjrosa wjrosa requested review from a team, annemirasol and malithsen and removed request for a team August 11, 2025 21:58
annemirasol
annemirasol reviewed Aug 12, 2025
View reviewed changes
malithsen
malithsen reviewed Aug 13, 2025
View reviewed changes
Copy link
Contributor

@malithsen malithsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge case: If the merchant disables all payment methods, we show the following error to the shopper

Image

I think we should hide the OC container if all payment methods are disabled.

@wjrosa
Copy link
Contributor Author

wjrosa commented Aug 14, 2025

Edge case: If the merchant disables all payment methods, we show the following error to the shopper

Image I think we should hide the OC container if all payment methods are disabled.

Good catch, @malithsen ! Hiding it in 467aef1

@annemirasol
Copy link
Contributor

Edge case: If the merchant disables all payment methods, we show the following error to the shopper
Image
I think we should hide the OC container if all payment methods are disabled.

Good catch, @malithsen ! Hiding it in 467aef1

This is what I see when there are no enabled payment methods:

Screenshot 2025-08-14 at 10 07 28 AM

@wjrosa wjrosa added this to the 9.9.0 milestone Aug 14, 2025
@wjrosa
Copy link
Contributor Author

wjrosa commented Aug 14, 2025

Edge case: If the merchant disables all payment methods, we show the following error to the shopper
Image
I think we should hide the OC container if all payment methods are disabled.

Good catch, @malithsen ! Hiding it in 467aef1

This is what I see when there are no enabled payment methods:

Screenshot 2025-08-14 at 10 07 28 AM

I saw a merge issue after your latest merge to develop. Fixed it in 44973f1. Any chance that was your issue? I cannot reproduce the error message in your screenshot. This is what I see:

Block checkout

Screenshot 2025-08-14 at 15 56 51

Shortcode checkout

Screenshot 2025-08-14 at 15 56 40

malithsen
malithsen reviewed Aug 14, 2025
View reviewed changes
Copy link
Contributor

@malithsen malithsen left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the "no payment methods available" error when all pms are disabled (as expected). Nice work!

However, Link is not presented on this branch, and it seems like the e2e failures are related to this.

@wjrosa wjrosa requested a review from malithsen August 18, 2025 15:19
@wjrosa
Copy link
Contributor Author

wjrosa commented Aug 18, 2025

Thanks, Malith! I have fixed the issue in 4cdbfac. As we spoke on Slack, e2e are passing now for the main suite, but still failing here for the OC ones 👀 (they work locally)

malithsen
malithsen approved these changes Aug 18, 2025
View reviewed changes
Copy link
Contributor

@malithsen malithsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me now! OC test timeouts don't seem related. I tested the flows manually in my local environment and it's working as expected. We should still have a look at those failures and address them in a separate PR.

annemirasol
annemirasol approved these changes Aug 19, 2025
View reviewed changes
Copy link
Contributor

@annemirasol annemirasol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, and possibly a separate issue, but there's a disconnect between what wp-admin and checkout says:

Screenshot 2025-08-19 at 8 57 25 AM

@wjrosa wjrosa enabled auto-merge (squash) August 19, 2025 18:17
@wjrosa wjrosa merged commit 23852c7 into develop Aug 19, 2025
39 of 40 checks passed
@wjrosa wjrosa deleted the fix/removing-card-requirement-for-oc branch August 19, 2025 18:25
@wjrosa
Copy link
Contributor Author

wjrosa commented Aug 19, 2025

Thanks, Anne! I have created STRIPE-678 to handle this specific issue, since it may require some additional investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annemirasol annemirasol annemirasol approved these changes

@malithsen malithsen malithsen approved these changes

Assignees

@wjrosa wjrosa

Labels
None yet
Projects
None yet
Milestone
9.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wjrosa @annemirasol @malithsen